LoopFi #2

Re: [WP-H4] PoolV3.sol Interest Cannot Be Distributed to Equity Holders

H4: This is an expected behavior, interest collected is minted as shares to a treasury contract and then distributed

Interest is not minted as shares to a treasury.

CDPVault.modifyCollateralAndDebt() in L379-384 uses calcDecrease(amount, position.debt, ...) to divide the amount paid by the creditor in L372 poolUnderlying.safeTransferFrom(creditor, address(pool), amount) into three parts:

  1. interestAccrued: see L611
  2. profitFromInterest: see L611
    • Also the last return value profit of calcDecrease(): see L613
  3. oldDebt - newDebt: see L633

CDPVault.modifyCollateralAndDebt() in L387 uses the above part 2 profit (i.e., profitFromInterest) as the second parameter in pool.repayCreditAccount(debtData.debt - newDebt, profit, 0)).

PoolV3.repayCreditAccount() L531 _mint(treasury, convertToShares(profit)); only distributes the above part 2 profit to the treasury by minting new shares.

The above part 3 "oldDebt - newDebt" offsets the previously borrowed principal, which is part of the total assets of the old shares.

The above part 1 interestAccrued is an additional asset transferred to the pool, separate from the old total assets of old shares and the profit of new shares.

Since PoolV3.withdraw() and PoolV3.redeem() use a fixed 1:1 share price, the share price is not affected by interestAccrued, leaving the above part 1 interestAccrued unclaimed.


On the other hand, according to PoolV3.sol#L504-506 and PoolV3.sol#L532-546, the profit in PoolV3.repayCreditAccount(repaidAmount, profit, loss) should be reserved to cover losses and should not be considered as interest to be distributed.

@@ 320,333 @@ /// @notice Modifies a Position's collateral and debt balances /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM, /// - that the Position is still safe after the modification, /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio, /// - that the msg.sender has the permission of the collateralizer to put up new collateral, /// - that the msg.sender has the permission of the creditor to settle debt with their credit, /// - that that the vault debt floor is exceeded /// - that the vault minimum collateralization ratio is met /// @param owner Address of the owner of the position /// @param collateralizer Address of who puts up or receives the collateral delta /// @param creditor Address of who provides or receives the credit delta for the debt delta /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad] /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or /// to settle (-) on this position [wad]
function modifyCollateralAndDebt( address owner, address collateralizer, address creditor, int256 deltaCollateral, int256 deltaDebt ) public {
@@ 341,355 @@ if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit;
if (deltaDebt > 0) {
@@ 357,364 @@ (newDebt, newCumulativeIndex) = calcIncrease( uint256(deltaDebt), // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); // U:[CM-10] pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) { uint256 maxRepayment = calcTotalDebt(debtData); uint256 amount = abs(deltaDebt); if (amount >= maxRepayment) { amount = maxRepayment; // U:[CM-11] } poolUnderlying.safeTransferFrom(creditor, address(pool), amount); if (amount == maxRepayment) {
@@ 375,377 @@ newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees;
} else { (newDebt, newCumulativeIndex, profit) = calcDecrease( amount, // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11] } else { newDebt = position.debt; newCumulativeIndex = debtData.cumulativeIndexNow; }
@@ 393,412 @@ if (deltaCollateral > 0) { uint256 amount = deltaCollateral.toUint256(); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = abs(deltaCollateral); token.safeTransfer(collateralizer, amount); } position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt); VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(newDebt, collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
@@ 573,593 @@ /// @dev Computes new debt principal and interest index (and other values) after decreasing debt /// - Debt comprises of multiple components which are repaid in the following order: /// quota update fees => quota interest => base interest => debt principal. /// New values for all these components depend on what portion of each was repaid. /// - Debt principal, for example, only decreases if all previous components were fully repaid /// - The new credit account's interest index stays the same if base interest was not repaid at all, /// is set to the current interest index if base interest was repaid fully, and is a solution to /// the equation `debt * (indexNow / indexLastUpdate - 1) - delta = debt * (indexNow / indexNew - 1)` /// when only `delta` of accrued interest was repaid /// @param amount Amount of debt to repay /// @param debt Debt principal before repayment /// @param cumulativeIndexNow The current interest index /// @param cumulativeIndexLastUpdate Credit account's interest index as of last update // @param cumulativeQuotaInterest Credit account's quota interest before repayment // @param quotaFees Accrued quota fees // @param feeInterest Fee on accrued interest (both base and quota) charged by the DAO /// @return newDebt Debt principal after repayment /// @return newCumulativeIndex Credit account's quota interest after repayment /// @return profit Amount of underlying tokens received as fees by the DAO // @return newCumulativeQuotaInterest Credit account's accrued quota interest after repayment // @return newQuotaFees Amount of unpaid quota fees left after repayment
function calcDecrease( uint256 amount, uint256 debt, uint256 cumulativeIndexNow, uint256 cumulativeIndexLastUpdate ) internal view returns (uint256 newDebt, uint256 newCumulativeIndex, uint256 profit) { uint256 amountToRepay = amount; if (amountToRepay != 0) { uint256 interestAccrued = calcAccruedInterest({
@@ 604,606 @@ amount: debt, cumulativeIndexLastUpdate: cumulativeIndexLastUpdate, cumulativeIndexNow: cumulativeIndexNow
}); // U:[CL-3] uint256 profitFromInterest = (interestAccrued * feeInterest) / PERCENTAGE_FACTOR; // U:[CL-3] if (amountToRepay >= interestAccrued + profitFromInterest) { amountToRepay -= interestAccrued + profitFromInterest; profit += profitFromInterest; // U:[CL-3] newCumulativeIndex = cumulativeIndexNow; // U:[CL-3] } else {
@@ 617,628 @@ // If amount is not enough to repay base interest + DAO fee, then it is split pro-rata between them uint256 amountToPool = (amountToRepay * PERCENTAGE_FACTOR) / (PERCENTAGE_FACTOR + feeInterest); profit += amountToRepay - amountToPool; // U:[CL-3] amountToRepay = 0; // U:[CL-3] newCumulativeIndex = (INDEX_PRECISION * cumulativeIndexNow * cumulativeIndexLastUpdate) / (INDEX_PRECISION * cumulativeIndexNow - (INDEX_PRECISION * amountToPool * cumulativeIndexLastUpdate) / debt); // U:[CL-3]
} } else { newCumulativeIndex = cumulativeIndexLastUpdate; // U:[CL-3] } newDebt = debt - amountToRepay; // U:[CL-3] }
    /// @notice Updates pool state to indicate debt repayment, can only be called by credit managers
    ///         after transferring underlying from a credit account to the pool.
    ///         - If transferred amount exceeds debt principal + base interest + quota interest,
    ///           the difference is deemed protocol's profit and the respective number of shares
    ///           is minted to the treasury.
    ///         - If, however, transferred amount is insufficient to repay debt and interest,
    ///           which may only happen during liquidation, treasury's shares are burned to
    ///           cover as much of the loss as possible.
    /// @param repaidAmount Amount of debt principal repaid
    /// @param profit Pool's profit in underlying after repaying
    /// @param loss Pool's loss in underlying after repaying
    /// @custom:expects Credit manager transfers underlying from a credit account to the pool before calling this function
    /// @custom:expects Profit/loss computed in the credit manager are cosistent with pool's implicit calculations
    function repayCreditAccount(
        uint256 repaidAmount,
        uint256 profit,
        uint256 loss
    )
        external
        override
        whenNotPaused // U:[LP-2A]
        nonReentrant // U:[LP-2B]
    {
        uint128 repaidAmountU128 = repaidAmount.toUint128();

        DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
        uint128 cmBorrowed = cmDebt.borrowed;
        if (cmBorrowed == 0) {
            revert CallerNotCreditManagerException(); // U:[LP-2C,14A]
        }

        if (profit > 0) {
            _mint(treasury, convertToShares(profit)); // U:[LP-14B]
        } else if (loss > 0) {
            address treasury_ = treasury;
            uint256 sharesInTreasury = balanceOf(treasury_);
            uint256 sharesToBurn = convertToShares(loss);
            if (sharesToBurn > sharesInTreasury) {
                unchecked {
                    emit IncurUncoveredLoss({
                        creditManager: msg.sender,
                        loss: convertToAssets(sharesToBurn - sharesInTreasury)
                    }); // U:[LP-14D]
                }
                sharesToBurn = sharesInTreasury;
            }
            _burn(treasury_, sharesToBurn); // U:[LP-14C,14D]
        }

        _updateBaseInterest({
            expectedLiquidityDelta: profit.toInt256() - loss.toInt256(),
            availableLiquidityDelta: 0,
            checkOptimalBorrowing: false
        }); // U:[LP-14B,14C,14D]

        _totalDebt.borrowed -= repaidAmountU128; // U:[LP-14B,14C,14D]
        cmDebt.borrowed = cmBorrowed - repaidAmountU128; // U:[LP-14B,14C,14D]

        emit Repay(msg.sender, repaidAmount, profit, loss); // U:[LP-14B,14C,14D]
    }
@@ 320,333 @@ /// @notice Modifies a Position's collateral and debt balances /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM, /// - that the Position is still safe after the modification, /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio, /// - that the msg.sender has the permission of the collateralizer to put up new collateral, /// - that the msg.sender has the permission of the creditor to settle debt with their credit, /// - that that the vault debt floor is exceeded /// - that the vault minimum collateralization ratio is met /// @param owner Address of the owner of the position /// @param collateralizer Address of who puts up or receives the collateral delta /// @param creditor Address of who provides or receives the credit delta for the debt delta /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad] /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or /// to settle (-) on this position [wad]
function modifyCollateralAndDebt( address owner, address collateralizer, address creditor, int256 deltaCollateral, int256 deltaDebt ) public {
@@ 341,355 @@ if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit;
if (deltaDebt > 0) {
@@ 357,364 @@ (newDebt, newCumulativeIndex) = calcIncrease( uint256(deltaDebt), // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); // U:[CM-10] pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) { uint256 maxRepayment = calcTotalDebt(debtData); uint256 amount = abs(deltaDebt); if (amount >= maxRepayment) { amount = maxRepayment; // U:[CM-11] } poolUnderlying.safeTransferFrom(creditor, address(pool), amount); if (amount == maxRepayment) {
@@ 375,377 @@ newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees;
} else { (newDebt, newCumulativeIndex, profit) = calcDecrease( amount, // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11] } else { newDebt = position.debt; newCumulativeIndex = debtData.cumulativeIndexNow; }
@@ 393,412 @@ if (deltaCollateral > 0) { uint256 amount = deltaCollateral.toUint256(); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = abs(deltaCollateral); token.safeTransfer(collateralizer, amount); } position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt); VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(newDebt, collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
@@ 573,593 @@ /// @dev Computes new debt principal and interest index (and other values) after decreasing debt /// - Debt comprises of multiple components which are repaid in the following order: /// quota update fees => quota interest => base interest => debt principal. /// New values for all these components depend on what portion of each was repaid. /// - Debt principal, for example, only decreases if all previous components were fully repaid /// - The new credit account's interest index stays the same if base interest was not repaid at all, /// is set to the current interest index if base interest was repaid fully, and is a solution to /// the equation `debt * (indexNow / indexLastUpdate - 1) - delta = debt * (indexNow / indexNew - 1)` /// when only `delta` of accrued interest was repaid /// @param amount Amount of debt to repay /// @param debt Debt principal before repayment /// @param cumulativeIndexNow The current interest index /// @param cumulativeIndexLastUpdate Credit account's interest index as of last update // @param cumulativeQuotaInterest Credit account's quota interest before repayment // @param quotaFees Accrued quota fees // @param feeInterest Fee on accrued interest (both base and quota) charged by the DAO /// @return newDebt Debt principal after repayment /// @return newCumulativeIndex Credit account's quota interest after repayment /// @return profit Amount of underlying tokens received as fees by the DAO // @return newCumulativeQuotaInterest Credit account's accrued quota interest after repayment // @return newQuotaFees Amount of unpaid quota fees left after repayment
function calcDecrease( uint256 amount, uint256 debt, uint256 cumulativeIndexNow, uint256 cumulativeIndexLastUpdate ) internal view returns (uint256 newDebt, uint256 newCumulativeIndex, uint256 profit) { uint256 amountToRepay = amount; if (amountToRepay != 0) { uint256 interestAccrued = calcAccruedInterest({
@@ 604,606 @@ amount: debt, cumulativeIndexLastUpdate: cumulativeIndexLastUpdate, cumulativeIndexNow: cumulativeIndexNow
}); // U:[CL-3] uint256 profitFromInterest = (interestAccrued * feeInterest) / PERCENTAGE_FACTOR; // U:[CL-3] if (amountToRepay >= interestAccrued + profitFromInterest) { amountToRepay -= interestAccrued + profitFromInterest; profit += profitFromInterest; // U:[CL-3] newCumulativeIndex = cumulativeIndexNow; // U:[CL-3] } else {
@@ 617,628 @@ // If amount is not enough to repay base interest + DAO fee, then it is split pro-rata between them uint256 amountToPool = (amountToRepay * PERCENTAGE_FACTOR) / (PERCENTAGE_FACTOR + feeInterest); profit += amountToRepay - amountToPool; // U:[CL-3] amountToRepay = 0; // U:[CL-3] newCumulativeIndex = (INDEX_PRECISION * cumulativeIndexNow * cumulativeIndexLastUpdate) / (INDEX_PRECISION * cumulativeIndexNow - (INDEX_PRECISION * amountToPool * cumulativeIndexLastUpdate) / debt); // U:[CL-3]
} } else { newCumulativeIndex = cumulativeIndexLastUpdate; // U:[CL-3] } newDebt = debt - amountToRepay; // U:[CL-3] }
    /// @notice Updates pool state to indicate debt repayment, can only be called by credit managers
    ///         after transferring underlying from a credit account to the pool.
    ///         - If transferred amount exceeds debt principal + base interest + quota interest,
    ///           the difference is deemed protocol's profit and the respective number of shares
    ///           is minted to the treasury.
    ///         - If, however, transferred amount is insufficient to repay debt and interest,
    ///           which may only happen during liquidation, treasury's shares are burned to
    ///           cover as much of the loss as possible.
    /// @param repaidAmount Amount of debt principal repaid
    /// @param profit Pool's profit in underlying after repaying
    /// @param loss Pool's loss in underlying after repaying
    /// @custom:expects Credit manager transfers underlying from a credit account to the pool before calling this function
    /// @custom:expects Profit/loss computed in the credit manager are cosistent with pool's implicit calculations
    function repayCreditAccount(
        uint256 repaidAmount,
        uint256 profit,
        uint256 loss
    )
        external
        override
        whenNotPaused // U:[LP-2A]
        nonReentrant // U:[LP-2B]
    {
        uint128 repaidAmountU128 = repaidAmount.toUint128();

        DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
        uint128 cmBorrowed = cmDebt.borrowed;
        if (cmBorrowed == 0) {
            revert CallerNotCreditManagerException(); // U:[LP-2C,14A]
        }

        if (profit > 0) {
            _mint(treasury, convertToShares(profit)); // U:[LP-14B]
        } else if (loss > 0) {
            address treasury_ = treasury;
            uint256 sharesInTreasury = balanceOf(treasury_);
            uint256 sharesToBurn = convertToShares(loss);
            if (sharesToBurn > sharesInTreasury) {
                unchecked {
                    emit IncurUncoveredLoss({
                        creditManager: msg.sender,
                        loss: convertToAssets(sharesToBurn - sharesInTreasury)
                    }); // U:[LP-14D]
                }
                sharesToBurn = sharesInTreasury;
            }
            _burn(treasury_, sharesToBurn); // U:[LP-14C,14D]
        }

        _updateBaseInterest({
            expectedLiquidityDelta: profit.toInt256() - loss.toInt256(),
            availableLiquidityDelta: 0,
            checkOptimalBorrowing: false
        }); // U:[LP-14B,14C,14D]

        _totalDebt.borrowed -= repaidAmountU128; // U:[LP-14B,14C,14D]
        cmDebt.borrowed = cmBorrowed - repaidAmountU128; // U:[LP-14B,14C,14D]

        emit Repay(msg.sender, repaidAmount, profit, loss); // U:[LP-14B,14C,14D]
    }

[WP-H2] Unexpected Waiver of Loan Interest When deltaDebt == 0 Due to Using debtData.cumulativeIndexNow Instead of position.cumulativeIndexLastUpdate for newCumulativeIndex

  • Expected: Loan status remains unchanged.
    • As neither principal nor interest has been repaid, the debt position interest index should remain unchanged. This allows for the accrual of interest from the last debt position modification to the current modifyCollateralAndDebt() call.
  • Current implementation: Interest is not repaid, yet the debt position interest index is modified, effectively equivalent to having repaid the interest.

https://github.com/LoopFi/loop-contracts/blob/1f8c6838bbd08cd92043d6cf7be1f8eafd6997e1/src/CDPVault.sol#L343-L436

@@ 343,354 @@ /// @notice Modifies a Position's collateral and debt balances /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM, /// - that the Position is still safe after the modification, /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio, /// - that the msg.sender has the permission of the collateralizer to put up new collateral, /// - that the msg.sender has the permission of the creditor to settle debt with their credit, /// - that that the vault debt floor is exceeded /// - that the vault minimum collateralization ratio is met /// @param owner Address of the owner of the position /// @param collateralizer Address of who puts up or receives the collateral delta /// @param creditor Address of who provides or receives the credit delta for the debt delta /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
/// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or /// to settle (-) on this position [wad] function modifyCollateralAndDebt( address owner, address collateralizer, address creditor, int256 deltaCollateral, int256 deltaDebt ) public { if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit; if (deltaDebt > 0) {
@@ 380,387 @@ (newDebt, newCumulativeIndex) = calcIncrease( uint256(deltaDebt), // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); // U:[CM-10] pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
} else if (deltaDebt < 0) {
@@ 389,410 @@ uint256 maxRepayment = calcTotalDebt(debtData); uint256 amount = abs(deltaDebt); if (amount >= maxRepayment) { amount = maxRepayment; // U:[CM-11] } poolUnderlying.safeTransferFrom(creditor, address(pool), amount); if (amount == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees; } else { (newDebt, newCumulativeIndex, profit) = calcDecrease( amount, // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
} else { newDebt = position.debt; newCumulativeIndex = debtData.cumulativeIndexNow; } if (deltaCollateral > 0) { uint256 amount = deltaCollateral.toUint256(); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = abs(deltaCollateral); token.safeTransfer(collateralizer, amount); } position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);
@@ 426,435 @@ VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(newDebt, collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}
    function _calcDebt(Position memory position) internal view returns (DebtData memory cdd) {
        uint256 index = pool.baseInterestIndex();
        cdd.debt = position.debt;
        cdd.cumulativeIndexNow = index;
        cdd.cumulativeIndexLastUpdate = position.cumulativeIndexLastUpdate;

        cdd.accruedInterest = calcAccruedInterest(cdd.debt, cdd.cumulativeIndexLastUpdate, index);

        cdd.accruedFees = (cdd.accruedInterest * feeInterest) / PERCENTAGE_FACTOR;
    }
    /// @notice Updates a position's collateral and debt balances
    /// @dev This is the only method which is allowed to modify a position's collateral and debt balances
    function _modifyPosition(
        address owner,
        Position memory position,
        uint256 newDebt,
        uint256 newCumulativeIndex,
        int256 deltaCollateral,
        uint256 totalDebt_
    ) internal returns (Position memory) {
        uint256 currentDebt = position.debt;
        // update collateral and debt amounts by the deltas
        position.collateral = add(position.collateral, deltaCollateral);
        position.debt = newDebt; // U:[CM-10,11]
        position.cumulativeIndexLastUpdate = newCumulativeIndex; // U:[CM-10,11]
        position.lastDebtUpdate = uint64(block.number); // U:[CM-10,11]

        // position either has no debt or more debt than the debt floor
        if (position.debt != 0 && position.debt < uint256(vaultConfig.debtFloor))
            revert CDPVault__modifyPosition_debtFloor();

        // store the position's balances
        positions[owner] = position;

        // update the global debt balance
        if (newDebt > currentDebt) {
            totalDebt_ = totalDebt_ + (newDebt - currentDebt);
        } else {
            totalDebt_ = totalDebt_ - (currentDebt - newDebt);
        }
        totalDebt = totalDebt_;

        if (address(rewardController) != address(0)) {
            rewardController.handleActionAfter(owner, position.debt, totalDebt_);
        }

        emit ModifyPosition(owner, position.debt, position.collateral, totalDebt_);

        return position;
    }

Recommendation

Change to:

    } else {
        newDebt = position.debt;
        newCumulativeIndex = position.cumulativeIndexLastUpdate;
    }
    function _calcDebt(Position memory position) internal view returns (DebtData memory cdd) {
        uint256 index = pool.baseInterestIndex();
        cdd.debt = position.debt;
        cdd.cumulativeIndexNow = index;
        cdd.cumulativeIndexLastUpdate = position.cumulativeIndexLastUpdate;

        cdd.accruedInterest = calcAccruedInterest(cdd.debt, cdd.cumulativeIndexLastUpdate, index);

        cdd.accruedFees = (cdd.accruedInterest * feeInterest) / PERCENTAGE_FACTOR;
    }
    /// @notice Updates a position's collateral and debt balances
    /// @dev This is the only method which is allowed to modify a position's collateral and debt balances
    function _modifyPosition(
        address owner,
        Position memory position,
        uint256 newDebt,
        uint256 newCumulativeIndex,
        int256 deltaCollateral,
        uint256 totalDebt_
    ) internal returns (Position memory) {
        uint256 currentDebt = position.debt;
        // update collateral and debt amounts by the deltas
        position.collateral = add(position.collateral, deltaCollateral);
        position.debt = newDebt; // U:[CM-10,11]
        position.cumulativeIndexLastUpdate = newCumulativeIndex; // U:[CM-10,11]
        position.lastDebtUpdate = uint64(block.number); // U:[CM-10,11]

        // position either has no debt or more debt than the debt floor
        if (position.debt != 0 && position.debt < uint256(vaultConfig.debtFloor))
            revert CDPVault__modifyPosition_debtFloor();

        // store the position's balances
        positions[owner] = position;

        // update the global debt balance
        if (newDebt > currentDebt) {
            totalDebt_ = totalDebt_ + (newDebt - currentDebt);
        } else {
            totalDebt_ = totalDebt_ - (currentDebt - newDebt);
        }
        totalDebt = totalDebt_;

        if (address(rewardController) != address(0)) {
            rewardController.handleActionAfter(owner, position.debt, totalDebt_);
        }

        emit ModifyPosition(owner, position.debt, position.collateral, totalDebt_);

        return position;
    }

[WP-H3] Incorrect calculation of loss amount

In the current implementation CDPVault.sol#L495-496, loss is merely the accumulated interest.

@@ 453,460 @@ /// @notice Liquidates a single unsafe positions by selling collateral at a discounted (`liquidationDiscount`) /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt /// the vault accumulates 'bad debt'. /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`. /// @param owner Owner of the position to liquidate /// @param repayAmount Amount the liquidator wants to repay [wad]
function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused { // validate params if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters(); // load configs VaultConfig memory config = vaultConfig; LiquidationConfig memory liqConfig_ = liquidationConfig; // load liquidated position Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); // load price and calculate discounted price uint256 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount); if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice(); // compute collateral to take, debt to repay and penalty to pay uint256 takeCollateral = wdiv(repayAmount, discountedPrice); uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty); uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // verify that the position is indeed unsafe if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice()), config.liquidationRatio)) revert CDPVault__liquidatePosition_notUnsafe(); // account for bad debt // TODO: review this uint256 loss; if (takeCollateral > position.collateral) { takeCollateral = position.collateral; repayAmount = wmul(takeCollateral, discountedPrice); penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // debt >= repayAmount if takeCollateral > position.collateral //deltaDebt = currentDebt; deltaDebt = debtData.debt; loss = calcTotalDebt(debtData) - deltaDebt; } // transfer the repay amount from the liquidator to the vault poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount); uint256 newDebt; uint256 profit; uint256 maxRepayment = calcTotalDebt(debtData); { uint256 newCumulativeIndex; if (deltaDebt == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees; } else { if (loss != 0) { profit = 0; newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; } else { (newDebt, newCumulativeIndex, profit) = calcDecrease( deltaDebt, // delta debt debtData.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray debtData.cumulativeIndexLastUpdate ); } } // update liquidated position position = _modifyPosition( owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); // U:[CM-11] // transfer the collateral amount from the vault to the liquidator // cash[msg.sender] += takeCollateral; token.safeTransfer(msg.sender, takeCollateral); // Mint the penalty from the vault to the treasury // cdm.modifyBalance(address(this), address(buffer), penalty); IPoolV3Loop(address(pool)).mintProfit(penalty); }

Recommendation

Change to:

uint256 loss;
if (takeCollateral > position.collateral) {
    takeCollateral = position.collateral;
    repayAmount = wmul(takeCollateral, discountedPrice);
    penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
    // debt >= repayAmount if takeCollateral > position.collateral
    //deltaDebt = currentDebt;
    loss = calcTotalDebt(debtData) - (repayAmount - penalty);
    deltaDebt = debtData.debt;
}
@@ 453,460 @@ /// @notice Liquidates a single unsafe positions by selling collateral at a discounted (`liquidationDiscount`) /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt /// the vault accumulates 'bad debt'. /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`. /// @param owner Owner of the position to liquidate /// @param repayAmount Amount the liquidator wants to repay [wad]
function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused { // validate params if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters(); // load configs VaultConfig memory config = vaultConfig; LiquidationConfig memory liqConfig_ = liquidationConfig; // load liquidated position Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); // load price and calculate discounted price uint256 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount); if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice(); // compute collateral to take, debt to repay and penalty to pay uint256 takeCollateral = wdiv(repayAmount, discountedPrice); uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty); uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // verify that the position is indeed unsafe if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice()), config.liquidationRatio)) revert CDPVault__liquidatePosition_notUnsafe(); // account for bad debt // TODO: review this uint256 loss; if (takeCollateral > position.collateral) { takeCollateral = position.collateral; repayAmount = wmul(takeCollateral, discountedPrice); penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // debt >= repayAmount if takeCollateral > position.collateral //deltaDebt = currentDebt; deltaDebt = debtData.debt; loss = calcTotalDebt(debtData) - deltaDebt; } // transfer the repay amount from the liquidator to the vault poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount); uint256 newDebt; uint256 profit; uint256 maxRepayment = calcTotalDebt(debtData); { uint256 newCumulativeIndex; if (deltaDebt == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees; } else { if (loss != 0) { profit = 0; newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; } else { (newDebt, newCumulativeIndex, profit) = calcDecrease( deltaDebt, // delta debt debtData.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray debtData.cumulativeIndexLastUpdate ); } } // update liquidated position position = _modifyPosition( owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); // U:[CM-11] // transfer the collateral amount from the vault to the liquidator // cash[msg.sender] += takeCollateral; token.safeTransfer(msg.sender, takeCollateral); // Mint the penalty from the vault to the treasury // cdm.modifyBalance(address(this), address(buffer), penalty); IPoolV3Loop(address(pool)).mintProfit(penalty); }

[WP-H4] Incorrect Bad Debt Determination

The current implementation of liquidatePosition() determines bad debt at L489 by checking if takeCollateral > position.collateral. However, takeCollateral is derived from L478 uint256 takeCollateral = wdiv(repayAmount, discountedPrice);, which is primarily controlled by the repayAmount parameter arbitrarily specified by the liquidator in the liquidatePosition() function (a liquidator can deliberately set a higher repayAmount).

The expected approach should be to compare the collateral amount, price, and total debt maxRepayment (i.e., calcTotalDebt(debtData)) to determine if it's a bad debt situation.

The current implementation may lead to:

  • Misidentifying a position as bad debt when it's not
    • Selling all collateral for a position that doesn't require it (depends on the penalty and liquidation discount and liquidatable health factor settings)
    • Deducting a loss from the treasury when unnecessary
  • Incorrectly processing an actual bad debt situation as if it weren't
    • Failing to close a position that should be closed
    • Adding profit to the treasury instead of deducting a loss when a loss should be recorded
@@ 453,460 @@ /// @notice Liquidates a single unsafe positions by selling collateral at a discounted (`liquidationDiscount`) /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt /// the vault accumulates 'bad debt'. /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`. /// @param owner Owner of the position to liquidate /// @param repayAmount Amount the liquidator wants to repay [wad]
function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused { // validate params if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();
@@ 465,471 @@ // load configs VaultConfig memory config = vaultConfig; LiquidationConfig memory liqConfig_ = liquidationConfig; // load liquidated position Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position);
// load price and calculate discounted price uint256 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount); if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice(); // compute collateral to take, debt to repay and penalty to pay uint256 takeCollateral = wdiv(repayAmount, discountedPrice); uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty); uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // verify that the position is indeed unsafe if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice()), config.liquidationRatio)) revert CDPVault__liquidatePosition_notUnsafe(); // account for bad debt // TODO: review this uint256 loss; if (takeCollateral > position.collateral) { takeCollateral = position.collateral; repayAmount = wmul(takeCollateral, discountedPrice); penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // debt >= repayAmount if takeCollateral > position.collateral //deltaDebt = currentDebt; deltaDebt = debtData.debt; loss = calcTotalDebt(debtData) - deltaDebt; } // transfer the repay amount from the liquidator to the vault poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount); uint256 newDebt; uint256 profit; uint256 maxRepayment = calcTotalDebt(debtData); { uint256 newCumulativeIndex; if (deltaDebt == maxRepayment) {
@@ 508,510 @@ newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees;
} else { if (loss != 0) { profit = 0; newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; } else { (newDebt, newCumulativeIndex, profit) = calcDecrease( deltaDebt, // delta debt debtData.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray debtData.cumulativeIndexLastUpdate ); } } // update liquidated position position = _modifyPosition( owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); // U:[CM-11] // transfer the collateral amount from the vault to the liquidator // cash[msg.sender] += takeCollateral; token.safeTransfer(msg.sender, takeCollateral); // Mint the penalty from the vault to the treasury // cdm.modifyBalance(address(this), address(buffer), penalty); IPoolV3Loop(address(pool)).mintProfit(penalty); }
@@ 453,460 @@ /// @notice Liquidates a single unsafe positions by selling collateral at a discounted (`liquidationDiscount`) /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt /// the vault accumulates 'bad debt'. /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`. /// @param owner Owner of the position to liquidate /// @param repayAmount Amount the liquidator wants to repay [wad]
function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused { // validate params if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();
@@ 465,471 @@ // load configs VaultConfig memory config = vaultConfig; LiquidationConfig memory liqConfig_ = liquidationConfig; // load liquidated position Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position);
// load price and calculate discounted price uint256 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount); if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice(); // compute collateral to take, debt to repay and penalty to pay uint256 takeCollateral = wdiv(repayAmount, discountedPrice); uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty); uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // verify that the position is indeed unsafe if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice()), config.liquidationRatio)) revert CDPVault__liquidatePosition_notUnsafe(); // account for bad debt // TODO: review this uint256 loss; if (takeCollateral > position.collateral) { takeCollateral = position.collateral; repayAmount = wmul(takeCollateral, discountedPrice); penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty); // debt >= repayAmount if takeCollateral > position.collateral //deltaDebt = currentDebt; deltaDebt = debtData.debt; loss = calcTotalDebt(debtData) - deltaDebt; } // transfer the repay amount from the liquidator to the vault poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount); uint256 newDebt; uint256 profit; uint256 maxRepayment = calcTotalDebt(debtData); { uint256 newCumulativeIndex; if (deltaDebt == maxRepayment) {
@@ 508,510 @@ newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedFees;
} else { if (loss != 0) { profit = 0; newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; } else { (newDebt, newCumulativeIndex, profit) = calcDecrease( deltaDebt, // delta debt debtData.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray debtData.cumulativeIndexLastUpdate ); } } // update liquidated position position = _modifyPosition( owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt ); } pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); // U:[CM-11] // transfer the collateral amount from the vault to the liquidator // cash[msg.sender] += takeCollateral; token.safeTransfer(msg.sender, takeCollateral); // Mint the penalty from the vault to the treasury // cdm.modifyBalance(address(this), address(buffer), penalty); IPoolV3Loop(address(pool)).mintProfit(penalty); }

[WP-M5] No approval granted to the flashLender

The repayment of the flashLoan (Flashlender.sol#L102) will revert as there was no allowance granted to the flashLender.

    function onFlashLoan(
        address /*initiator*/,
        address /*token*/,
        uint256 /*amount*/,
        uint256 /*fee*/,
        bytes calldata data
    ) external returns (bytes32) {
@@ 385,411 @@ if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender(); (LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode( data, (LeverParams, address, uint256) ); // perform a pre swap from arbitrary token to collateral token if necessary if (leverParams.auxSwap.assetIn != address(0)) { bytes memory auxSwapData = _delegateCall( address(swapAction), abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap) ); upFrontAmount = abi.decode(auxSwapData, (uint256)); } // swap stablecoin to collateral bytes memory swapData = _delegateCall( address(swapAction), abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap) ); uint256 swapAmountOut = abi.decode(swapData, (uint256)); // deposit collateral and handle any CDP specific actions uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut); // derive the amount of normal debt from the amount of Stablecoin swapped uint256 addDebt = leverParams.primarySwap.amount;
// add collateral and debt ICDPVault(leverParams.vault).modifyCollateralAndDebt( leverParams.position, address(this), address(this), toInt256(collateral), toInt256(addDebt) ); return CALLBACK_SUCCESS; }
   function flashLoan(
        IERC3156FlashBorrower receiver,
        address token,
        uint256 amount,
        bytes calldata data
    ) external override nonReentrant returns (bool) {
        if (token != address(underlyingToken)) revert Flash__flashLoan_unsupportedToken();
        uint256 fee = wmul(amount, protocolFee);
        uint256 total = amount + fee;

        pool.lendCreditAccount(amount, address(receiver));

        emit FlashLoan(address(receiver), token, amount, fee);

        if (receiver.onFlashLoan(msg.sender, token, amount, fee, data) != CALLBACK_SUCCESS)
            revert Flash__flashLoan_callbackFailed();

        // reverts if not enough Stablecoin have been send back
        underlyingToken.transferFrom(address(receiver), address(pool), total);
        pool.repayCreditAccount(total - fee, fee, 0);

        return true;
    }
    function onFlashLoan(
        address /*initiator*/,
        address /*token*/,
        uint256 /*amount*/,
        uint256 /*fee*/,
        bytes calldata data
    ) external returns (bytes32) {
@@ 385,411 @@ if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender(); (LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode( data, (LeverParams, address, uint256) ); // perform a pre swap from arbitrary token to collateral token if necessary if (leverParams.auxSwap.assetIn != address(0)) { bytes memory auxSwapData = _delegateCall( address(swapAction), abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap) ); upFrontAmount = abi.decode(auxSwapData, (uint256)); } // swap stablecoin to collateral bytes memory swapData = _delegateCall( address(swapAction), abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap) ); uint256 swapAmountOut = abi.decode(swapData, (uint256)); // deposit collateral and handle any CDP specific actions uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut); // derive the amount of normal debt from the amount of Stablecoin swapped uint256 addDebt = leverParams.primarySwap.amount;
// add collateral and debt ICDPVault(leverParams.vault).modifyCollateralAndDebt( leverParams.position, address(this), address(this), toInt256(collateral), toInt256(addDebt) ); return CALLBACK_SUCCESS; }
   function flashLoan(
        IERC3156FlashBorrower receiver,
        address token,
        uint256 amount,
        bytes calldata data
    ) external override nonReentrant returns (bool) {
        if (token != address(underlyingToken)) revert Flash__flashLoan_unsupportedToken();
        uint256 fee = wmul(amount, protocolFee);
        uint256 total = amount + fee;

        pool.lendCreditAccount(amount, address(receiver));

        emit FlashLoan(address(receiver), token, amount, fee);

        if (receiver.onFlashLoan(msg.sender, token, amount, fee, data) != CALLBACK_SUCCESS)
            revert Flash__flashLoan_callbackFailed();

        // reverts if not enough Stablecoin have been send back
        underlyingToken.transferFrom(address(receiver), address(pool), total);
        pool.repayCreditAccount(total - fee, fee, 0);

        return true;
    }

[WP-N6] Incorrect deltaDebt in ModifyCollateralAndDebt event when deltaDebt > maxRepayment

When deltaDebt exceeds maxRepayment, the repayment amount is adjusted to maxRepayment. However, the ModifyCollateralAndDebt event still uses the original deltaDebt value.

function modifyCollateralAndDebt(
        address owner,
        address collateralizer,
        address creditor,
        int256 deltaCollateral,
        int256 deltaDebt
    ) public {
@@ 370,386 @@ if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit; int256 quotaRevenueChange;
if (deltaDebt > 0) { (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease( uint256(deltaDebt), // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); // U:[CM-10] position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt); pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20] } else if (deltaDebt < 0) { uint256 maxRepayment = calcTotalDebt(debtData); uint256 amount = abs(deltaDebt); if (amount >= maxRepayment) { amount = maxRepayment; // U:[CM-11] } poolUnderlying.safeTransferFrom(creditor, address(pool), amount); uint128 newCumulativeQuotaInterest; if (amount == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedInterest; newCumulativeQuotaInterest = 0; } else { (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease( amount, // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate, debtData.cumulativeQuotaInterest ); } quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt)); pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11] position.cumulativeQuotaInterest = newCumulativeQuotaInterest; position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; } if (deltaCollateral > 0) { uint256 amount = deltaCollateral.toUint256(); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = abs(deltaCollateral); token.safeTransfer(collateralizer, amount); }
@@ 435,449 @@ position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt); VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(newDebt, collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); if (quotaRevenueChange != 0) { IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15] }
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt); }
function modifyCollateralAndDebt(
        address owner,
        address collateralizer,
        address creditor,
        int256 deltaCollateral,
        int256 deltaDebt
    ) public {
@@ 370,386 @@ if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit; int256 quotaRevenueChange;
if (deltaDebt > 0) { (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease( uint256(deltaDebt), // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate ); // U:[CM-10] position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt); pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20] } else if (deltaDebt < 0) { uint256 maxRepayment = calcTotalDebt(debtData); uint256 amount = abs(deltaDebt); if (amount >= maxRepayment) { amount = maxRepayment; // U:[CM-11] } poolUnderlying.safeTransferFrom(creditor, address(pool), amount); uint128 newCumulativeQuotaInterest; if (amount == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedInterest; newCumulativeQuotaInterest = 0; } else { (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease( amount, // delta debt position.debt, debtData.cumulativeIndexNow, // current cumulative base interest index in Ray position.cumulativeIndexLastUpdate, debtData.cumulativeQuotaInterest ); } quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt)); pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11] position.cumulativeQuotaInterest = newCumulativeQuotaInterest; position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; } if (deltaCollateral > 0) { uint256 amount = deltaCollateral.toUint256(); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = abs(deltaCollateral); token.safeTransfer(collateralizer, amount); }
@@ 435,449 @@ position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt); VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(newDebt, collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); if (quotaRevenueChange != 0) { IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15] }
emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt); }